An optional non-list key makes isList Maybe, not No by zonuexe · Pull Request #6025 · phpstan/phpstan-src

zonuexe · 2026-07-08T18:00:14Z

A ConstantArrayType whose only list-incompatible keys are optional can still be a list when those keys are absent — array{a?: string} admits [], and array_is_list([]) === true. So its isList must be Maybe, not No. Previously any string / negative / gap key forced isList = No regardless of optionality, so array_is_list() was reported as always false on such shapes.

This is the dual of #12725 (there, shapes that also admit non-list values wrongly report isList yes; here, shapes that also admit list values wrongly report no).

Parts:

ConstantArrayTypeBuilder — an optional list-incompatible key degrades isList Yes→Maybe (No stays No) via a new markNonListKey(), replacing four unconditional createNo() sites.
ArrayType::isSuperTypeOf() — a possibly-empty constant array always admits [], which is a subtype of every array type, so the relationship is at worst Maybe, never a definite No. This lets the ($value is list ? true : false) conditional of array_is_list() resolve to bool for possibly-empty shapes (e.g. array{a?: string}) instead of false.
ConstantArrayType::makeList() — now that gap/string optional keys yield isList Maybe, intersecting a sealed such shape with list keeps only the contiguous 0..m prefix (the keys that can actually appear in a list) rather than collapsing to *NEVER*. Unsealed extras may fill the gaps, so there every key is kept.
ConstantArrayType::mergeWith() / legacyMergeWith() — merging widens keys present in only one side into optional keys, so the result can admit list realizations neither input did (array{a: 1}|array{b: 2} → array{a?: 1, b?: 2} admits []). The list-ness of a sealed merged shape is now recomputed from the shape via inferIsListFromShape() instead of the too-strict $this->isList->and($otherArray->isList); two pure lists still merge into a list. This is the same merge-side fix as Degrade isList to maybe for optional non-list keys in array shapes and shape merges #6026, folded in here — thanks to the parallel analysis there. (The one deliberate difference from Degrade isList to maybe for optional non-list keys in array shapes and shape merges #6026 is makeList() above: this PR projects invalid sealed list-shapes to their valid form, e.g. list{0: string, 1: int, 2?: string, 4?: string} → list{0: string, 1: int, 2?: string}, rather than keeping the unreachable key; see the tradeoff note on Degrade isList to maybe for optional non-list keys in array shapes and shape merges #6026.)

/** @param array{a?: string} $a */         // array_is_list($a): false -> bool
/** @param array{0: int, a?: string} $b */ // array_is_list($b): false -> bool

The existing array-shape-list-optional.php expectations change accordingly: an invalid sealed list-shape like list{0: string, 1: int, 2?: string, 4?: string} now resolves to its valid projection array{0: string, 1: int, 2?: string}.

Closes phpstan/phpstan#14938

zonuexe · 2026-07-08T18:20:21Z

+	public function narrowing(array $optStr): void
+	{
+		if (array_is_list($optStr)) {
+			assertType('list{}&list', $optStr);


Before this fix array{a?: string} had isList = No, so this branch was *NEVER* (unreachable). With isList now Maybe, the empty-array realisation survives the is list intersection, so the branch is reachable and narrows to the empty list.

zonuexe · 2026-07-08T18:20:30Z

+		} else {
+			// array_is_list()'s false branch is not narrowed, so `a` stays optional
+			// here rather than being refined to array{a: string}.
+			assertType('array{a?: string}', $optStr);


The a? here (rather than array{a: string}) is pre-existing and unrelated to this fix: array_is_list()'s false branch is not refined. It stays a? for sealed shapes too — e.g. even array{0: string, a?: string} keeps a? in the false branch on current stable.

zonuexe · 2026-07-08T18:54:50Z

Update: I folded the mergeWith() / legacyMergeWith() fix (and the oracle-validated inferIsListFromShape() helper) from #6026 into this PR — that merge-side case (array{a: 1}|array{b: 2} widening to array{a?: 1, b?: 2}, which admits []) was missing here. Credit for that part goes to #6026.

The only remaining difference between the two PRs is the projection of an invalid sealed list-shape in makeList() (drop the unreachable key here vs. keep it in #6026) — see the A/B tradeoff note I left on #6026 (comment).

phpstan-bot · 2026-07-08T19:11:42Z

This pull request has been marked as ready for review.

phpstan-bot · 2026-08-06T02:48:47Z

This pull request has been marked as ready for review.

staabm · 2026-08-07T08:39:17Z

+		// A sealed shape with an optional key past the gap at 1 (2? here): key 2 can
+		// never appear in a list, so makeList() drops it and projects to the prefix.
+		BleedingEdgeToggle::withBleedingEdge(true, function (): void {
+			$array = $this->buildShape([[0, new IntegerType(), false], [2, new StringType(), true]]);
+			$this->assertSame(TrinaryLogic::createMaybe()->describe(), $array->isList()->describe());
+			$this->assertSame('array{int}', $array->makeList()->describe(VerbosityLevel::precise()));
+		});
+
+		// Unsealed: extras may fill the gap, so key 2 is reachable and every key is kept.
+		BleedingEdgeToggle::withBleedingEdge(false, function (): void {
+			$array = $this->buildShape([[0, new IntegerType(), false], [2, new StringType(), true]]);
+			$this->assertSame(TrinaryLogic::createMaybe()->describe(), $array->isList()->describe());
+			$this->assertSame('array{0: int, 2?: string}', $array->makeList()->describe(VerbosityLevel::precise()));
+		});


can we use a data-provider to ease reading the test? atm its hard to see where the diff is between with/without bleeding edge.

same in the other test

Done. Both tests are now data providers. testMakeListProjectsSealedShapeButKeepsUnsealedKeys has one row per sealedness: the input shape is the same, only the expected makeList() projection differs. testMergeWithRecomputesListnessOfSealedShape has one named row per {bleeding edge, shapes, expected isList}, so you can compare the with and without cases row by row. The test method applies the toggle itself, because data providers run before the container enables bleeding edge. I re-verified by manual mutation that the rows kill the mutants of the $naiveIsList->or(infer…) combinator in both mergeWith() and legacyMergeWith().

staabm · 2026-08-07T08:39:58Z

+
+	public function testMergeWithTreatsNumericStringKeyAsIntWhenRecomputingListness(): void
+	{
+		BleedingEdgeToggle::withBleedingEdge(true, function (): void {


do we need a BleedingEdgeToggle::withBleedingEdge(false,...) variant of this test?

Yes, added. inferIsListFromShape() has two call sites: mergeWith() (bleeding edge, sealed by the explicit-never unsealed marker) and legacyMergeWith() (unsealed === null). The test is now a data provider with both variants. The new withBleedingEdge(false) row goes through legacyMergeWith() and fails with Maybe instead of Yes without the toArrayKey() normalization, so it covers the second call site.

staabm

I like the new data-provider tests a lot more. thank you

A ConstantArrayType whose only list-incompatible keys are optional can still be a list when those keys are absent (`array{a?: string}` admits `[]`), so its isList must be Maybe, not No. Previously any string / negative / gap key forced No regardless of optionality, which made `array_is_list()` report "always false" on such shapes. - ConstantArrayTypeBuilder: an optional list-incompatible key degrades isList Yes to Maybe (No stays No) via markNonListKey(). - ArrayType::isSuperTypeOf(): a possibly-empty constant array always admits `[]`, a subtype of every array type, so the relationship is at worst Maybe — never a definite No. This lets the `($value is list)` conditional of array_is_list() resolve to bool for possibly-empty shapes instead of false. - ConstantArrayType::makeList(): now that gap/string optional keys yield isList Maybe, intersecting a sealed such shape with list keeps only the contiguous 0..m prefix (the keys that can actually appear in a list) instead of collapsing to *NEVER*. Closes phpstan/phpstan#14938

…ally

…ergeWith with a numeric-string key

staabm · 2026-08-08T13:46:39Z

@SanderMuller here another PR you could review. thank you

SanderMuller · 2026-08-08T19:13:17Z

Reviewed this one — I checked it out and ran it rather than reading only. Short version: the reasoning holds up, the fix does what it claims, and I could not make it report anything wrong.

The premise is right. array{a?: string} admits [], and array_is_list([]) === true, so maybe is the correct trinary answer where no was returned. The same goes for the isSuperTypeOf() part, which I initially read as too permissive: array<int, string> and array{a?: string} both contain [], so the two overlap — the honest answer is maybe, and the previous no claimed they were disjoint when they are not.

Behaviour, 81e06a583 vs the PR head (dumpType(array_is_list($a)), level 9):

param	before	after
`array{a?: string}`	`false`	`bool`
`array{0: int, a?: string}`	`false`	`bool`
`array{1?: string}`	`false`	`bool`
`array{a: string}`	`false`	`false`
`array{a?: string}` after `$a !== []`	`false`	`false`
`list<string>`	`true`	`true`

The last three are the ones I was most interested in — a required non-list key, a shape narrowed to non-empty, and a genuine list all keep their old answer, so the new maybe does not leak into places where no/yes was already correct.

inferIsListFromShape() I worked through by hand on the shapes that matter (array{a?: string}, array{a: string}, array{0?: int, 1: string}, array{0: int, 1?: string}, a plain list) and the reachable-prefix-length construction gives the right answer in each. Normalizing through toArrayKey() so a numeric key that arrives as a ConstantStringType still counts as continuing the list is a good defensive touch — shapes written as array{'1'?: string} are already normalized to array{1?: string} before they get here, but the helper does not have to rely on that.

makeList() projecting list{0: string, 1: int, 2?: string, 4?: string} down to array{0: string, 1: int, 2?: string} is right and strictly better than the old *NEVER*: the list-valid realizations really are exactly {0,1} and {0,1,2}, so *NEVER* was claiming an impossibility that isn't one.

Gates, all on the PR head: full suite green (21302 tests / 96876 assertions, and again green run sequentially rather than through paratest), make phpstan clean, and no measurable memory change on the optional-key benches — finite-types-optional-keys-blowup.php 58.495mb → 58.505mb, bug-7140.php 65.391mb → 65.362mb, measured through phpbench itself.

On the four red jobs — I looked at each, none of them is this PR as far as I can tell:

Test (PHP 8.5) is the phpbench step. Every row in the comparison is inflated (+3% to +28% on mode, including benches this PR cannot touch, and one at −0.29%), which reads as a loaded runner rather than a targeted regression. bug-15061.php showing ERR is just that the bench was added in 81e06a583 itself, so it has no stored baseline yet.
Run with Turbo Extension (macos) is PHPStan process crashed because it reached configured PHP memory limit: 450M.
Integration tests (windows) is Nette\IOException: Unable to include '…/nette.configurator/Container_….php' in the runner temp dir.
Tests with old PHPUnit (8.1, ubuntu) is the interesting one: three IntersectionTypeTest::testIsAcceptedBy data sets expecting Maybe and getting No. I could not reproduce it on 8.4 or 8.5, in parallel or sequentially, over the whole suite. What argues against it being this change: the same PHP 8.1 passes on windows while only the ubuntu job fails, 7.4 and 8.0 pass on both, and the same test failed the same way on an unrelated PR (Avoid warning about non-nullable class-property in traits. #6100, an isset() rules change) very recently. With executionOrder="random" in phpunit.xml that pattern looks order-dependent. I would still re-run that job before merging, because it is fragile in exactly the corner this PR touches, and if it survives a re-run it deserves a proper look rather than my guess.

Two things I would ask, neither blocking:

In mergeWith() the sealed case is $naiveIsList->or(self::inferIsListFromShape(...)). For a sealed shape the inferred value is computed from the merged keys themselves, so it is the more informative of the two — is the or there to guard a case where the inference is less precise than $this->isList->and($other->isList)? If not, using the inferred value directly would be simpler, and would also avoid keeping a yes that the shape does not support.
inferIsListFromShape() is O(n²) in the number of keys (the reachable-length set grows with each optional key). It only runs on sealed merges and I measured no change on the optional-key blowup benches, so this is a note rather than an objection — just worth knowing it is there if someone later hits a pathological shape.

Last thing, for whoever merges: #6026 is still open and fixes the same issue, and this PR now contains its merge-side fix, so the two need deciding together rather than separately. The remaining difference is the makeList() projection, and I think dropping the unreachable key (this PR) is the better of the two, for the reason above.

zonuexe force-pushed the fix-islist-optional-keys branch from e27c6d8 to 9fa048d Compare July 8, 2026 18:19

zonuexe commented Jul 8, 2026

View reviewed changes

zonuexe mentioned this pull request Jul 8, 2026

Degrade isList to maybe for optional non-list keys in array shapes and shape merges #6026

Open

zonuexe marked this pull request as draft July 8, 2026 18:48

zonuexe marked this pull request as ready for review July 8, 2026 19:11

zonuexe force-pushed the fix-islist-optional-keys branch from 702067f to 25d81d1 Compare July 20, 2026 06:07

zonuexe marked this pull request as draft August 6, 2026 01:59

zonuexe force-pushed the fix-islist-optional-keys branch 2 times, most recently from e3e6293 to 291f84b Compare August 6, 2026 02:43

zonuexe marked this pull request as ready for review August 6, 2026 02:48

staabm reviewed Aug 6, 2026

View reviewed changes

Comment thread src/Type/Constant/ConstantArrayType.php

zonuexe force-pushed the fix-islist-optional-keys branch 3 times, most recently from 91450e0 to 501c39d Compare August 7, 2026 02:11

staabm force-pushed the fix-islist-optional-keys branch from 8928a85 to d9f6883 Compare August 7, 2026 08:19

staabm reviewed Aug 7, 2026

View reviewed changes

staabm force-pushed the fix-islist-optional-keys branch from 01f9c5a to dc9a7e6 Compare August 7, 2026 09:06

staabm approved these changes Aug 7, 2026

View reviewed changes

staabm requested a review from ondrejmirtes August 7, 2026 09:10

zonuexe added 3 commits August 8, 2026 15:46

Use a named argument instead of restating the isList default position…

212bcf5

…ally

Turn the isList recompute tests into data providers and cover legacyM…

badee13

…ergeWith with a numeric-string key

staabm force-pushed the fix-islist-optional-keys branch from dc9a7e6 to badee13 Compare August 8, 2026 13:46

Navigation Menu

Uh oh!

An optional non-list key makes isList Maybe, not No - #6025

An optional non-list key makes isList Maybe, not No#6025
zonuexe wants to merge 3 commits into
phpstan:2.2.xfrom
zonuexe:fix-islist-optional-keys

zonuexe commented Jul 8, 2026 •

edited

Loading

Uh oh!

zonuexe Jul 8, 2026

Uh oh!

zonuexe Jul 8, 2026

Uh oh!

zonuexe commented Jul 8, 2026 •

edited

Loading

Uh oh!

phpstan-bot commented Jul 8, 2026

Uh oh!

phpstan-bot commented Aug 6, 2026

Uh oh!

Uh oh!

staabm Aug 7, 2026 •

edited

Loading

Uh oh!

zonuexe Aug 7, 2026

Uh oh!

staabm Aug 7, 2026 •

edited

Loading

Uh oh!

zonuexe Aug 7, 2026

Uh oh!

staabm left a comment

Uh oh!

staabm commented Aug 8, 2026

Uh oh!

SanderMuller commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

zonuexe commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zonuexe Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

zonuexe Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

zonuexe commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phpstan-bot commented Jul 8, 2026

Uh oh!

phpstan-bot commented Aug 6, 2026

Uh oh!

Uh oh!

staabm Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zonuexe Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

staabm Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zonuexe Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

staabm left a comment

Choose a reason for hiding this comment

Uh oh!

staabm commented Aug 8, 2026

Uh oh!

SanderMuller commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zonuexe commented Jul 8, 2026 •

edited

Loading

zonuexe commented Jul 8, 2026 •

edited

Loading

staabm Aug 7, 2026 •

edited

Loading

staabm Aug 7, 2026 •

edited

Loading